TIKA-4766: typed Document parse contract for tika-grpc by krickert · Pull Request #2921 · apache/tika

krickert · 2026-07-01T22:38:33Z

Summary

Follow-up to #2916, reshaped per the review there. Instead of mirroring Tika's open metadata taxonomy in protobuf (~5k lines of proto, per-format messages), this PR types the thing that is actually stable: the parsed document. One small contract — document.proto is 208 lines — and format specifics live in per-parser mapping code, never in the wire.

FetchAndParseReply.fields (map<string,string>, field 2, now reserved) is replaced by FetchAndParseReply.document.

How this answers the #2916 review

Concern from #2916	Where it landed
"11k lines to nail down maybe 80% of an open set"	208-line contract; whole PR is ~3.9k lines, most of it mapper code + tests
Clients rebuild when metadata definitions change	A metadata key is now data (`extra` tail), not schema — add/rename/retype a Tika `Property` and no client regenerates anything
Lossless catch-all as source of truth	`Document.extra` carries every Tika key, multivalue-preserving; a test asserts nothing is dropped
Special handling only for DC + core props	`DocumentMetadata` types only the bounded cross-format fields (title, authors, dates as `Timestamp`, counts, dimensions, rights)
Break it into individually reviewable tasks	This PR is the contract only; see "Deliberately not in this PR"

The shape

Content tree: markdown (the same render ToMarkdownContentHandler already produces since TIKA-4730) plus blocks — that markdown parsed once, format-agnostically, into a structured tree of headings/paragraphs/lists/tables/code blocks/inline runs (CommonMark + GFM, a spec that does not churn). This is what a downstream NLP/RAG/embeddings consumer actually wants: typed tables and sections, not a string to re-parse.
Typed common metadata: DocumentMetadata, grouped by concern, not by source format. Dates are Timestamps, counts are ints — not strings that 12 language clients each re-parse.
Tagged tail: extra — every remaining key, typed only where Tika's own Property declares a type (integer/real/boolean/date), string otherwise, never guessed.
embedded recurses: a PDF with an embedded image is a parent Document with a fully typed child — no forcing two formats into one bucket (this was the oneof problem from TIKA-4766: Typed parse response grpc #2916).
Adding a format = adding a DocumentTransformer (see tika-grpc-mapper/docs/EXTENSIONS.md); PdfDocumentTransformer is 65 lines and the wire contract does not move.

Deliberately not in this PR (follow-ups, each its own PR)

Pluggable external parsers: registering a third-party gRPC service whose output rides along on the Document as a google.protobuf.Any — so wildly different result shapes (e.g. a document-layout model's tree) never require Tika to model them. Built and tested on a branch; kept out to keep this reviewable.
A Markdown parser for .md input files (separate JIRA).
Richer typed fields, if and only if real cross-format demand appears — they'd be additive optional fields, compatible both directions.

Open decisions where reviewer preference wins

Tail shape: repeated MetadataField with a typed value oneof (as implemented) vs the map<string, StringList> suggested in TIKA-4766: Typed parse response grpc #2916. The typed-where-declared tail preserves types without guessing; the map is maximally churn-proof. Swapping is a one-message change — happy to go either way.
markdown + blocks both: today both ship (string render + structured tree). If payload size matters, a per-request flag choosing one is easy.
Hard removal vs staged: fields is hard-removed (4.0, nothing consumes it yet); can switch to deprecate-then-remove if preferred.

Client migration

Before	After
`fields["X-TIKA:content"]`	`document.markdown` (or walk `document.blocks`)
`fields["Content-Type"]`	`document.content_type`
Ad hoc title/author/date strings	`document.metadata.title` / `.authors` / `.created` (`Timestamp`)
Any other key	`document.extra` (typed by declared `Property` type, string otherwise)

Test plan

./mvnw -pl tika-grpc-api,tika-grpc-mapper,tika-grpc test — green (transformer tests against real parse fixtures per format, block-tree tests, DocumentBuilder envelope/status/embedded tests, server tests reading FetchAndParseReply.document)
tika-grpc-api jar bundles META-INF/org.apache.tika.grpc.v1.descriptors (verified: contains document.proto)
e2e tika-grpc-e2e-test compiles against the new API
CI

Downstream context: this contract is what the OpenNLP gRPC work (OPENNLP-1833) will consume as input — Tika parse → typed document → NLP/embeddings without re-parsing strings.

…t contract One small, stable proto instead of a message per source format. Contract (tika-grpc-api, ~200 lines of proto): - Document: content as a structured markdown block tree (headings, paragraphs, lists, tables, code blocks, inline runs -- CommonMark + GFM, the same markdown ToMarkdownContentHandler already renders), plus `markdown` as the authoritative rendered form. - DocumentMetadata: a small bounded set of typed common fields grouped by concern, not by source format (title/authors/keywords/languages, created and modified as Timestamps, page/word/character counts, dimensions, rights). - Tagged tail: `extra` carries every remaining Tika key losslessly and multivalue-preserving, typed only where Tika's own Property declares a type (integer/real/boolean/date) and string otherwise -- never guessed. - `embedded` recurses: a parent PDF and an embedded image are each a fully typed Document. - `format_category` is a cheap routing hint; cross-cutting concerns such as Creative Commons rights coexist with it rather than fighting a oneof. Format specifics live in per-parser DocumentTransformer code (tika-grpc-mapper), not in the wire contract: adding or changing a parser never touches the proto, and metadata churn lands in the mapper and the tagged tail, so a new or renamed metadata key never forces a client rebuild. Server (tika-grpc): - FetchAndParseReply.document (field 5) replaces the fields map (field 2, now reserved). TikaGrpcServerImpl maps parse output via DocumentBuilder. Modules: tika-grpc-api (proto + generated messages + bundled FileDescriptorSet), tika-grpc-mapper (DocumentBuilder, per-format transformers, markdown block-tree builder), both listed in tika-bom. Tests: transformer tests against real parse fixtures per format, block tree round-trip tests, DocumentBuilder envelope/status/embedded tests, server tests updated to read FetchAndParseReply.document.

krickert · 2026-07-02T12:24:35Z

Follow-up work is broken out into TIKA-4771 (pluggable external parsers) and TIKA-4772 (document event streaming) rather than growing this PR.

krickert · 2026-07-03T04:38:27Z

@nddipiazza @tballison what do you think of this design instead? far less fields - ability to roll your own protobuf model in the future. Best of both worlds. Document structure is very markdown-friendly. I'll make the output of this be able to be the input of the grpc OpenNLP grpc server.

nddipiazza

Review: e2e test coverage for the new typed `Document` contract

Reviewed the design overall — the shift from mirroring Tika's metadata taxonomy in protobuf to a small, stable document.proto (208 lines) plus per-parser mapper code is a solid answer to the #2916 feedback. A few things worth addressing before merge:

Testing gap: e2e coverage does not exercise the new contract

The only change in tika-e2e-tests/tika-grpc is a 2-line fix in HandlerTypeTest.java:

- String htmlContent = htmlReply.getFieldsMap().get("X-TIKA:content");
+ String htmlContent = htmlReply.getDocument().getMarkdown();

(same for the TEXT handler case). This is a compile-fix to keep the pre-existing assertion working against the new API — it only proves document.markdown is non-empty over a real gRPC round-trip for two handler types.

It does not exercise, end-to-end, through the live server:

DocumentMetadata typed fields (title/authors/dates/counts) — no e2e assertion anywhere
extra tagged-tail typing (int/bool/date/string) — no e2e assertion
blocks structured content tree (headings/tables/lists/code) — untouched by any e2e test
embedded recursion for container formats (e.g. an Office doc with an embedded image) — no e2e coverage at all
format_category routing hint — untouched

The other e2e tests (FileSystemFetcherTest, IgniteConfigStoreTest, ExternalTestBase) only ever inspect getFetchKey()/getStatus() on FetchAndParseReply — none of them look at getDocument(), so the bulk-corpus/streaming/ignite e2e paths give zero signal on the new typed contract.

The real proof of correctness for the mapping logic lives entirely in tika-grpc-mapper's unit tests (DocumentBuilderTest, MarkdownBlockTreeBuilderTest, one test class per transformer), which feed fixture-derived Metadata/markdown into DocumentBuilder in-process. That's good for the mapping logic itself, but it bypasses the actual gRPC wire serialization, the live server (TikaGrpcServerImpl), the pipes client, and fetcher plumbing entirely.

Ask: add (or extend HandlerTypeTest) at least one e2e case per format that fetches a real file through the live gRPC server and asserts on document.metadata (a couple of typed fields), document.extra (at least one tagged key), and one document.embedded case (e.g. an Office/PDF file with an embedded image) — not just that markdown is non-empty. Right now this new, larger surface area has no live-server coverage at all.

Other findings from code inspection

Likely-dead error path: DocumentBuilder.build()'s primary == null branch (returns FAILED with "No metadata returned from parse") looks unreachable in production — the only caller, TikaGrpcServerImpl, always passes a non-null Metadata (tikaMetadata = new Metadata() as fallback when emitData()/metadata list is empty). Worth double-checking this is intentional, since a real fetch failure with no emit data currently produces an "empty-but-success-shaped" Document rather than hitting this explicit error branch.
Duplication: 7 of 8 format transformers (Generic, Pdf, Html, Image, Office, Rtf, Epub) repeat an identical block mapping TITLE/DESCRIPTION/CREATOR/SUBJECT/LANGUAGE/CREATED/MODIFIED. Consider factoring this into a shared helper in TransformSupport called by each transformer, keeping only the format-specific additions per-transformer.
Minor: parseTimeMs in TikaGrpcServerImpl measures fetch+parse combined (timer starts before pipesClient.process()), though the field's doc/intent reads as parse-only time.

Nice work on the overall shape of the contract — the main blocker from my read is the e2e coverage gap above.

nddipiazza

Overall this is a solid design and a much better-scoped answer to the #2916 feedback: a small stable 208-line proto, format logic pushed into per-parser Java transformers, and a lossless typed+tagged tail. Requesting changes mainly on test coverage for the new contract, plus a couple of correctness/maintainability issues found while reading the mapper code (see inline comments).

E2E coverage (the review's main ask): HandlerTypeTest now exercises the typed contract through the live gRPC server -- typed DocumentMetadata fields, tagged-tail typing (pdf:encrypted arrives as a boolean, not a string), the block tree, format_category, and embedded recursion for a container docx. New fixtures: testPDF.pdf, test_recursive_embedded.docx. Bug found by the new e2e tests: Property.get() only sees property constants whose declaring class is initialized, and the parse runs in a forked JVM -- so tagged-tail typing silently degraded to strings in the server. MetadataTagger now force-initializes the tika-core property vocabularies so tagging is deterministic. Also fixed while here: the e2e configs now opt in to allowComponentManagement/allowPerRequestConfig, without which every e2e test dies with PERMISSION_DENIED since the TIKA-4764 security gating. DocumentBuilder: the primary==null branch is now reachable -- the server passes null when the pipes result carried no metadata instead of an empty Metadata that produced a success-shaped Document on fetch failures. Status still derives from the pipes status (EMPTY_OUTPUT with no metadata is a success, not an error). Dedup: the common field block (title/description/authors/keywords/ languages/created/modified) now lives in TransformSupport.mapCommonFields, used by six transformers; Office/RTF keep meta:keyword as their keyword source via an overload. RTF gains the dc:language mapping it had drifted away from. Image stays bespoke on purpose (IPTC/EXIF sources). Renamed ParseStatus.parse_time_ms to fetch_parse_time_ms: the timer wraps the whole pipesClient.process() round trip, and fetch+parse are not separable server-side.

…contract

krickert · 2026-07-07T19:03:10Z

@nddipiazza great feedback..

Review: e2e test coverage for the new typed Document contract

Reviewed the design overall — the shift from mirroring Tika's metadata taxonomy in protobuf to a small, stable document.proto (208 lines) plus per-parser mapper code is a solid answer to the #2916 feedback. A few things worth addressing before merge:

Testing gap: e2e coverage does not exercise the new contract

The only change in tika-e2e-tests/tika-grpc is a 2-line fix in HandlerTypeTest.java:
- String htmlContent = htmlReply.getFieldsMap().get("X-TIKA:content");
+ String htmlContent = htmlReply.getDocument().getMarkdown();
(same for the TEXT handler case). This is a compile-fix to keep the pre-existing assertion working against the new API — it only proves document.markdown is non-empty over a real gRPC round-trip for two handler types.

It does not exercise, end-to-end, through the live server:

DocumentMetadata typed fields (title/authors/dates/counts) — no e2e assertion anywhere

extra tagged-tail typing (int/bool/date/string) — no e2e assertion

blocks structured content tree (headings/tables/lists/code) — untouched by any e2e test

embedded recursion for container formats (e.g. an Office doc with an embedded image) — no e2e coverage at all

format_category routing hint — untouched

The other e2e tests (FileSystemFetcherTest, IgniteConfigStoreTest, ExternalTestBase) only ever inspect getFetchKey()/getStatus() on FetchAndParseReply — none of them look at getDocument(), so the bulk-corpus/streaming/ignite e2e paths give zero signal on the new typed contract.

The real proof of correctness for the mapping logic lives entirely in tika-grpc-mapper's unit tests (DocumentBuilderTest, MarkdownBlockTreeBuilderTest, one test class per transformer), which feed fixture-derived Metadata/markdown into DocumentBuilder in-process. That's good for the mapping logic itself, but it bypasses the actual gRPC wire serialization, the live server (TikaGrpcServerImpl), the pipes client, and fetcher plumbing entirely.

Ask: add (or extend HandlerTypeTest) at least one e2e case per format that fetches a real file through the live gRPC server and asserts on document.metadata (a couple of typed fields), document.extra (at least one tagged key), and one document.embedded case (e.g. an Office/PDF file with an embedded image) — not just that markdown is non-empty. Right now this new, larger surface area has no live-server coverage at all.

Other findings from code inspection

Likely-dead error path: DocumentBuilder.build()'s primary == null branch (returns FAILED with "No metadata returned from parse") looks unreachable in production — the only caller, TikaGrpcServerImpl, always passes a non-null Metadata (tikaMetadata = new Metadata() as fallback when emitData()/metadata list is empty). Worth double-checking this is intentional, since a real fetch failure with no emit data currently produces an "empty-but-success-shaped" Document rather than hitting this explicit error branch.

Duplication: 7 of 8 format transformers (Generic, Pdf, Html, Image, Office, Rtf, Epub) repeat an identical block mapping TITLE/DESCRIPTION/CREATOR/SUBJECT/LANGUAGE/CREATED/MODIFIED. Consider factoring this into a shared helper in TransformSupport called by each transformer, keeping only the format-specific additions per-transformer.

Minor: parseTimeMs in TikaGrpcServerImpl measures fetch+parse combined (timer starts before pipesClient.process()), though the field's doc/intent reads as parse-only time.

Nice work on the overall shape of the contract — the main blocker from my read is the e2e coverage gap above.

My bad; the e2e suite really was only asserting "markdown is non-empty". Added three live-server cases to HandlerTypeTest:

PDF (testPDF.pdf): asserts typed document.metadata (title, page count), a boolean-typed document.extra key (pdf:encrypted), document.blocks, and format_category.
HTML (sample.html): asserts typed title, FORMAT_WEB, and a string-typed extra key.
Embedded (test_recursive_embedded.docx): asserts FORMAT_OFFICE, typed created/modified dates, and that document.embedded recurses with per-child metadata.

Two things fell out of writing these:

The extra-tail typing was actually broken over the live server - MetadataTagger relies on Property.get(key), but Tika's property registry is populated lazily by class init, so in the server JVM pdf:encrypted came back as a string, not a boolean. The unit tests never caught it because they happen to load the vocabulary classes first. Fixed by force-loading the tika-core metadata vocabularies in a static initializer in MetadataTagger, and the new e2e test now pins the behavior.
The e2e configs needed grpc.allowComponentManagement / allowPerRequestConfig opt-ins after TIKA-4764 landed on main, otherwise every test dies with PERMISSION_DENIED - updated the three config JSONs.

Note the CI e2e job doesn't currently execute these (the -pl tika-e2e-tests -am verify invocation only builds the aggregator POM and no plugins are provisioned) - that's a pre-existing infra gap, but the suite passes locally against a live server.

krickert · 2026-07-07T19:04:42Z

Thanks for the review @nddipiazza, all four points addressed:

e2e tests now exercise the typed contract over a live server (PDF/HTML/embedded DOCX), which flushed out and fixed a real bug: lazy Property registry init made typed extra keys degrade to strings in the server JVM.
The primary == null branch is now reachable (server passes null on empty output) and correctly treats EMPTY_OUTPUT as success.
Common field mapping deduped into TransformSupport.mapCommonFields.
Renamed parse_time_ms to fetch_parse_time_ms to match what's actually measured.

Also merged latest main (the lombok removal and split-packages changes). Full build plus mapper, grpc, and e2e suites pass locally.

krickert force-pushed the TIKA-4766-document-contract branch 2 times, most recently from c1de042 to 1d2d5bd Compare July 1, 2026 23:55

krickert mentioned this pull request Jul 2, 2026

TIKA-4770: Add a Markdown parser with structured, lossless XHTML output #2922

Merged

4 tasks

krickert force-pushed the TIKA-4766-document-contract branch from 1d2d5bd to da51be9 Compare July 2, 2026 01:41

nddipiazza reviewed Jul 7, 2026

View reviewed changes

nddipiazza requested changes Jul 7, 2026

View reviewed changes

krickert added 2 commits July 7, 2026 14:35

wqMerge remote-tracking branch 'origin/main' into TIKA-4766-document-…

234247b

…contract

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

TIKA-4766: typed Document parse contract for tika-grpc#2921

TIKA-4766: typed Document parse contract for tika-grpc#2921
krickert wants to merge 3 commits into
apache:mainfrom
ai-pipestream:TIKA-4766-document-contract

krickert commented Jul 1, 2026

Uh oh!

krickert commented Jul 2, 2026

Uh oh!

krickert commented Jul 3, 2026

Uh oh!

nddipiazza left a comment

Uh oh!

nddipiazza left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krickert commented Jul 7, 2026

Review: e2e test coverage for the new typed `Document` contract

Testing gap: e2e coverage does not exercise the new contract

Other findings from code inspection

Uh oh!

krickert commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

krickert commented Jul 1, 2026

Summary

How this answers the #2916 review

The shape

Deliberately not in this PR (follow-ups, each its own PR)

Open decisions where reviewer preference wins

Client migration

Test plan

Uh oh!

krickert commented Jul 2, 2026

Uh oh!

krickert commented Jul 3, 2026

Uh oh!

nddipiazza left a comment

Choose a reason for hiding this comment

Review: e2e test coverage for the new typed Document contract

Testing gap: e2e coverage does not exercise the new contract

Other findings from code inspection

Uh oh!

nddipiazza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krickert commented Jul 7, 2026

Review: e2e test coverage for the new typed Document contract

Testing gap: e2e coverage does not exercise the new contract

Other findings from code inspection

Uh oh!

krickert commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Review: e2e test coverage for the new typed `Document` contract

Review: e2e test coverage for the new typed `Document` contract